bpo-36302: Sort list of sources by bmwiedemann · Pull Request #12341 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Could there be negative consequences to the sorting?
I think the two most common idioms for defining sources in setup.py are 1) an explicit list with fixed order (e.g. sources = ["lib/thing.c", "lib/utils.c"]) and 2) a glob result.
Is it ok in 100% of cases to change the order for case 1) ?
Let's say the case 2 people seem to not have noticed any negative consequences from random order, so I would assume that it is fine for case 1, too. I also mailed this to our expert to be on the safe side.
Our compiler expert said, order does not matter for .so files. It only matters when using .a files in case of duplicate symbols - but those are for static linking and not for dynamic linking as required by python.
ping. Anything left to do on my side?
ping. Still wanting this. Hoping for @vstinner or who else could have a look.
Because I found another of the infinite callers.
bmwiedemann added a commit to bmwiedemann/pyreadstat that referenced this pull request
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to modify where self.sources is used rather than sorting Extension constructor? For example, read_setup_file() function seems to modify the sources attribute afterwards.
Maybe modify CCompiler.compile()?
patching build_ext would probably work (edit: does not need test updates). read_setup_file code looks deterministic enough (unless someone autogenerates the setup file nondeterministically).
CCompiler.compile is overridden in msvccompiler.py so might not catch all cases.
link_shared_object might also be a good place to patch instead, because after all it is the linking order of .o files into .so that matters - it just happens to follow the source file order.
moved the sorting into build_ext
Thanks for updated PR: sorting in build_extension() looks more reliable than sorting in Extension constructor.
lisroach pushed a commit to lisroach/cpython that referenced this pull request
DinoV pushed a commit to DinoV/cpython that referenced this pull request
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request
