bpo-43976: add vendor config by FFY00 · Pull Request #25718 · 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

Conversation85 Commits9 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 }})

The "Check if generated files are up to date" test is failing because of unfortunate timing when merging other PRs (GH-25315, GH-25687).
It's OK to ignore it. Sorry for the inconvenience.
To make it pass, you can merge the master branch.

No worries, I haven't committed the generated files anyway. I purposedly excluded generated files from the commit to make it easier to review. I still have to add documentation to the commit, and then I will push a commit updating the generated files, I can rebase when I do that. The only reason I opened this PR now anyway was that I am not gonna be home this afternoon and wanted to add the missing bits from my laptop.
Before I mark it ready for review I will be sure to rebase (I usually do) 😊

Please use autoconf 2.69 to regenerate configure scripts and helpers. Most core devs are on platforms that do not have autoconf 2.71 yet.

The venv failure seems to be because we are hitting pypa/pip#9617.

FFY00 marked this pull request as ready for review

April 29, 2021 16:29

FFY00 changed the title~~bpo-41282: add vendor config~~ bpo-43976: add vendor config

Apr 29, 2021

I think this needs a mailing-list or forum discussion.

I would have notified the people in pypa/pip#9617 but that is locked, maybe @uranusjr or @pradyunsg can ping them. (But as all design discussions, please use a proper python forum and not a PR)

I have opened https://discuss.python.org/t/mechanism-for-distributors-to-add-site-install-schemes-to-python-installations/8467.

A note about the PR. Currently, the site module does not import sysconfig, but it contains two copied functions from it. As far as I understand, this is just a micro optimization, the only two sysconfig functions it needs are pretty simple and do not depend on other sysconfig code or other modules.
This PR, however, needs a significant part of sysconfig, so it imports it.

I did some bechmarks to see if we would notice any performance difference, but the results are pretty negligible.

With the PR:

$ ./python -m timeit 'import site'
2000000 loops, best of 5: 167 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 174 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 166 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 176 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 184 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 176 nsec per loop

In master:

$ ./python -m timeit 'import site'
2000000 loops, best of 5: 176 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 179 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 171 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 176 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 177 nsec per loop
$ ./python -m timeit 'import site'
2000000 loops, best of 5: 176 nsec per loop

If this PR goes in, I will open a new PR removing the two sysconfig functions that the site module vendors.

Can you build python from scratch if sysconfig imports site?
What about cross-compilation? (which has unclear support status)

For the stuff with the pip release, the PR that needs merging is pypa/pip#9912. Once that's merged and released, things should get fixed.

I have applied #25761 to fix the tests, github should be able to merge this without issue after that PR goes in.

@uranusjr can you take a look at the last commit (the get_preferred_schemes one)?