Simplify Modules/Setup{,.dist,.local} #76611

JulienPalard · 2017-12-27T11:15:04Z

BPO	32430
Nosy	@Yhg1s, @warsaw, @nascheme, @pitrou, @vstinner, @ericvsmith, @xdegaye, @vadmium, @koobs, @yan12125, @JulienPalard, @tonybaloney
PRs	bpo-32430: Add the `--enable-use-setup-dist` configure option #5062 bpo-32430: Rename Modules/Setup.dist to Modules/Setup #8229 bpo-32430: During 'configure', don't copy Setup.dist to Setup. #8260 bpo-32429: Abort compilation on outdated Modules/Setup file. #5015

^{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']

The text was updated successfully, but these errors were encountered:

JulienPalard · 2017-12-27T11:15:03Z

Related to: https://bugs.python.org/issue32429

The bahavior of Modules/Setup{,.dist,.local} looks inherited from an old time:

A Setup.dist exist
./configure copies it to Setup (cp $srcdir/Modules/Setup.dist Modules/Setup)
./configure installs a dummy Modules/Setup.local file
One can add an optional things to Setup.local
One can have to modify Setup itself (to modify things)

From this point in time, Setup is newer than Setup.dist (as copied from it), the following can happen:

The Setup file is manually modified, it's still newer than Setup.dist, it does not change the situation
The Setup.dist file gets updated (via a git checkout or git merge), the Makefile detects it and warn the user:
- Either the user modified the Setup file and don't want it to be overridden
- Either the user didn't modified it and just want the updated version

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:

Xavier de Gaye: Use Setup.dist instead of Setup when Setup is missing (https://bugs.python.org/issue32429#msg309078)
Just keep a single versionned Setup file, let those who modify it use branches to commit their modifications

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 · 2017-12-27T11:54:28Z

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.

+1 for this.

xdegaye · 2017-12-31T10:31:28Z

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-dist configure 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 · 2017-12-31T12:55:47Z

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 · 2017-12-31T13:03:19Z

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 · 2017-12-31T14:01:13Z

and I've never seen it done by anyone else

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 · 2017-12-31T14:06:21Z

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 · 2017-12-31T14:06:51Z

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 ?

Yes, I think so. Also my hope is to remove it in 3.8 :-)

xdegaye · 2017-12-31T14:36:45Z

Yes 3.8 and this leaves plenty of time for writing some documentation on the build process :-)

JulienPalard · 2018-01-07T11:27:20Z

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 Setup file is not enough, can someone give a clear explanation on this point?

vadmium · 2018-01-08T01:00:23Z

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 · 2018-01-09T13:49:46Z

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.

vstinner · 2018-01-20T01:07:18Z

I like the idea of removing the Modules/Setup.dist => Modules/Setup copy which removes immediately the annoying warning (bpo-32429).

... that one can modify and commit

I'm not sure about the "commit" part. I see Modules/Setup as a configuration file. Python provides a default configuration, fine.

Why not adding an option in Makefile or configure to allow to specify a different path to a custom Setup file?

./configure --setup-file=google_uber_cool_setup_file && make

pitrou · 2018-01-20T13:17:50Z

Why not adding an option in Makefile or configure to allow to specify a different path to a custom Setup file?

Because we don't want to maintain more config options? Really, I don't to want to further complicate our build process.

vstinner · 2018-01-20T13:22:44Z

Because we don't want to maintain more config options? Really, I don't to want to further complicate our build process.

I don't see how adding an option to make one path configurable would make the build process more complicated.

It seems like Xavier and Thomas do use Modules/Setup for their need, and for me it's strange to have to modify Modules/Setup (or Modules/Setup.dist) if it's tracked by Git. If I modify it, IMHO it's a configuration file, and it shouldn't be part of Python code base. It's fine to have a default configuration, but if you don't want to use the default, just pass the path to your own config. No?

pitrou · 2018-01-20T13:25:19Z

Le 20/01/2018 à 14:22, STINNER Victor a écrit :

I don't see how adding an option to make one path configurable would make the build process more complicated.

Do you think implementing and maintaing an option has no cost?

It seems like Xavier and Thomas do use Modules/Setup for their need, and for me it's strange to have to modify Modules/Setup (or Modules/Setup.dist) if it's tracked by Git.

Distributors routinely modify files that are tracked by git.
Modules/Setup doesn't have to be an exception.

See Anaconda example I posted above. Also you can look up packaging by
RedHat or any other distro.

JulienPalard · 2018-01-20T14:28:25Z

I don't see a use case where one want to edit a file without having his work being tracked by git, can someone shed a light to me on this?

I understand that to modify a single configuration point (a path, whatever) a configure option is the right way and is not versioned by git, but Modules/Setup is a file, not a flag.

But, I assumed the whole time Modules/Setup modifications are not trivial, but the result of some work worth versioning, unlike a single configure option, am I wrong?

xdegaye · 2018-01-21T12:56:30Z

I fully concur with all points in Victor msg310345.

xdegaye · 2018-01-21T13:49:52Z

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 · 2018-02-06T23:00:30Z

Taking arguments into account, this solution may work:

Continue to distribute a Setup.dist
Add a configure option to change it, as Victor said --setup-file should do it, the default value should
be Setup.dist
Remove the cp Setup.dist Setup

This 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 · 2018-07-10T20:00:48Z

I've created a PR that renames Setup.dist to Setup and removes extraneous logic.

nascheme · 2018-07-11T18:12:59Z

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 · 2018-07-12T16:45:42Z

I removed the "makesetup_fallback_dist.txt". PR-8260 is a more polished implementation of the same idea.

pitrou · 2018-07-16T17:03:14Z

New changeset 961d54c by Antoine Pitrou in branch 'master':
bpo-32430: Rename Modules/Setup.dist to Modules/Setup (GH-8229)
961d54c

pitrou · 2018-07-16T17:04:41Z

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 · 2018-07-16T20:33:26Z

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 · 2019-05-06T15:34:14Z

PR #49312 is still open in GitHub, is this PR still required?

vstinner · 2019-05-06T15:37:44Z

PR #49312 is still open in GitHub, is this PR still required?

It's not. I closed it, thanks.

JulienPalard added build The build process and cross-build 3.7 (EOL) end of life labels Dec 27, 2017

pitrou added the type-feature A feature request or enhancement label Dec 27, 2017

pitrou added 3.8 only security fixes and removed 3.7 (EOL) end of life labels Jul 10, 2018

pitrou closed this as completed Jul 16, 2018

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify Modules/Setup{,.dist,.local} #76611

Simplify Modules/Setup{,.dist,.local} #76611

JulienPalard commented Dec 27, 2017

JulienPalard commented Dec 27, 2017

pitrou commented Dec 27, 2017

xdegaye mannequin commented Dec 31, 2017

pitrou commented Dec 31, 2017

pitrou commented Dec 31, 2017

xdegaye mannequin commented Dec 31, 2017

pitrou commented Dec 31, 2017

pitrou commented Dec 31, 2017

xdegaye mannequin commented Dec 31, 2017

JulienPalard commented Jan 7, 2018

vadmium commented Jan 8, 2018

Yhg1s commented Jan 9, 2018

vstinner commented Jan 20, 2018

pitrou commented Jan 20, 2018

vstinner commented Jan 20, 2018

pitrou commented Jan 20, 2018

JulienPalard commented Jan 20, 2018

xdegaye mannequin commented Jan 21, 2018

xdegaye mannequin commented Jan 21, 2018

JulienPalard commented Feb 6, 2018

pitrou commented Jul 10, 2018

nascheme commented Jul 11, 2018

nascheme commented Jul 12, 2018

pitrou commented Jul 16, 2018

pitrou commented Jul 16, 2018

vstinner commented Jul 16, 2018

tonybaloney mannequin commented May 6, 2019

vstinner commented May 6, 2019

Simplify Modules/Setup{,.dist,.local} #76611

Simplify Modules/Setup{,.dist,.local} #76611

Comments

JulienPalard commented Dec 27, 2017

JulienPalard commented Dec 27, 2017

pitrou commented Dec 27, 2017

xdegaye mannequin commented Dec 31, 2017

pitrou commented Dec 31, 2017

pitrou commented Dec 31, 2017

xdegaye mannequin commented Dec 31, 2017

pitrou commented Dec 31, 2017

pitrou commented Dec 31, 2017

xdegaye mannequin commented Dec 31, 2017

JulienPalard commented Jan 7, 2018

vadmium commented Jan 8, 2018

Yhg1s commented Jan 9, 2018

vstinner commented Jan 20, 2018

pitrou commented Jan 20, 2018

vstinner commented Jan 20, 2018

pitrou commented Jan 20, 2018

JulienPalard commented Jan 20, 2018

xdegaye mannequin commented Jan 21, 2018

xdegaye mannequin commented Jan 21, 2018

JulienPalard commented Feb 6, 2018

pitrou commented Jul 10, 2018

nascheme commented Jul 11, 2018

nascheme commented Jul 12, 2018

pitrou commented Jul 16, 2018

pitrou commented Jul 16, 2018

vstinner commented Jul 16, 2018

tonybaloney mannequin commented May 6, 2019

vstinner commented May 6, 2019