homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: "python -S" should be robust against e.g. "from site import addsitedir"
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: brett.cannon, carljm, eric.araujo, python-dev
Priority: normal Keywords: patch

Created on 2011年03月17日 19:47 by carljm, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
87df1d37c88e.diff carljm, 2011年03月17日 19:47 review
ebe5760afa08.diff carljm, 2011年03月21日 17:48 review
test_site-11591.diff eric.araujo, 2011年06月09日 15:26 review
Repositories containing patches
https://bitbucket.org/carljm/cpython-robust-site
https://bitbucket.org/carljm/cpython-robust-site
Messages (16)
msg131281 - (view) Author: Carl Meyer (carljm) * Date: 2011年03月17日 19:46
If python is run with the -S flag, that declares the intent of the user to not have site-specific additions to sys.path.
However, some code in that process may have a legitimate need for a function defined in site.py - for instance, addsitedir. But the act of importing site.py, as a side effect, adds the standard site-specific directories to sys.path.
python -S would be more useful and reliable if it prevented importing site from automatically making the sys.path additions. There is no loss of flexibility here, as user code could still explicitly call site.main() to achieve all of the current side-effects of "import site".
The fix is a one-liner, and is in the linked hg repository.
msg131286 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年03月17日 21:31
Thanks. Would you mind adding tests in test_site?
msg131294 - (view) Author: Carl Meyer (carljm) * Date: 2011年03月17日 22:35
Adding a test is easier said than done. The behavior change here depends on python being run with -S. Currently test_site skips itself if the test suite is run with -S, and if I remove that skip it crashes under -S.
Options as I see it:
1. Declare this one-liner correct by inspection. It doesn't break any existing tests.
2. Add a new test file (test_no_site.py?) that only runs with -S and tests that importing something from site doesn't trigger sys.path additions. This seems like the most reasonable test, but I'm not sure how useful it is, since I doubt most people ever try running the test suite with -S.
3. Make the fix more complicated such that it uses an intermediary variable which can be mocked (unlike sys.flags.no_site, which is read-only), and then add a test which mocks this variable, temporarily removes "site" from sys.modules, tries importing it again, and checks whether main() is called. This creates a complex test which is highly coupled to the implementation in site.py, but would be run under normal conditions (without -S).
Which option do you prefer?
msg131322 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年03月18日 09:08
Fair argument. Brett is the author of recent changes in site, let him decide.
Brett: Would you agree to 1)?
msg131536 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011年03月20日 20:15
This is what I get for trying to clean up site.py years ago. =)
I'm fine with the change as long as there is a very clear Misc/NEWS message that the semantics on import have changed (and obviously this is not backported).
msg131580 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年03月21日 00:21
I think this requires a note in NEWS but also in Doc/library/site.rst. Carl, would you like to make a new changeset with that edition in your repo? (It will also provide a test for the Create Patch button after.)
msg131681 - (view) Author: Carl Meyer (carljm) * Date: 2011年03月21日 17:47
Added documentation to Doc/library/site.rst and Misc/NEWS.
msg131706 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年03月21日 23:12
Looks great, thank you. I think I’ll also add a docstring to main before committing, now that the function is publicly documented.
Did you have to manually click "Create Patch" to make roundup generate it? Did you try first to click on the button of the existing repo before adding a new repo entry? (Still learning how to use it, thanks for experimenting along :)
Brett: Thanks for the review. If you don’t comment negatively on the doc change, I will commit this.
(Side concern: the module does not define __all__, even though only 4 functions and 4 constants are officially documented. I’d like to define __all__, but the recentish huge thread on public/private APIs scared me.)
msg131709 - (view) Author: Carl Meyer (carljm) * Date: 2011年03月22日 00:05
> Did you have to manually click "Create Patch" to make roundup generate it? 
Yes - the first time too.
> Did you try first to click on the button of the existing repo before adding a new repo entry?
That would probably have worked fine. The "Remote hg repo" field was just empty when I made my latest comment, so I filled it in again. Wasn't sure if it would duplicate, or be smart enough to tell they were the same repo, or what. I guess it duplicated :/
msg131748 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年03月22日 13:54
> The "Remote hg repo" field was just empty when I made my latest comment
Looks like this field is always empty: its goal is to add a repo, just like the File field is always empty unless you add a file. The existing files and repositories are however listed underneath the form, so your screen just probably did not go low enough for you to see that the repo was still listed.
msg131761 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2011年03月22日 18:33
Doc changes seem fine to me.
msg131848 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011年03月23日 03:53
New changeset a364719e400a by Éric Araujo in branch 'default':
Do not touch sys.path when site is imported and python was started with -S.
http://hg.python.org/cpython/rev/a364719e400a 
msg131850 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年03月23日 04:09
Committed with small wording changes and more docs. Thank you, and good luck for cpythonv!
msg134568 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011年04月27日 14:37
New changeset 9a1ca0062950 by Éric Araujo in branch 'default':
Add versionchanged for a364719e400a (#11591)
http://hg.python.org/cpython/rev/9a1ca0062950 
msg136968 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011年05月26日 14:51
New changeset 6ee5443773cb by Éric Araujo in branch 'default':
Also add versionchanged directive to the function doc (#11591)
http://hg.python.org/cpython/rev/6ee5443773cb 
msg137988 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011年06月09日 15:26
Now that site can be imported without side effects under -S, I think the tests could be updated: they don’t have to be all skipped under -S. See attached patch.
History
Date User Action Args
2022年04月11日 14:57:14adminsetgithub: 55800
2011年06月09日 15:26:13eric.araujosetfiles: + test_site-11591.diff

messages: + msg137988
2011年05月26日 14:51:44python-devsetmessages: + msg136968
2011年04月27日 14:37:35python-devsetmessages: + msg134568
2011年03月23日 04:09:31eric.araujosetstatus: open -> closed
nosy: brett.cannon, carljm, eric.araujo, python-dev
messages: + msg131850

resolution: fixed
stage: resolved
2011年03月23日 03:53:54python-devsetnosy: + python-dev
messages: + msg131848
2011年03月23日 00:39:21terry.reedysetfiles: - ebe5760afa08.diff
nosy: brett.cannon, carljm, eric.araujo
2011年03月23日 00:38:57terry.reedysetfiles: + ebe5760afa08.diff
nosy: brett.cannon, carljm, eric.araujo
2011年03月22日 18:33:38brett.cannonsetassignee: brett.cannon -> eric.araujo
messages: + msg131761
nosy: brett.cannon, carljm, eric.araujo
2011年03月22日 13:54:11eric.araujosetnosy: brett.cannon, carljm, eric.araujo
messages: + msg131748
2011年03月22日 02:23:15brett.cannonsetassignee: eric.araujo -> brett.cannon
nosy: brett.cannon, carljm, eric.araujo
2011年03月22日 00:05:04carljmsetnosy: brett.cannon, carljm, eric.araujo
messages: + msg131709
2011年03月21日 23:12:34eric.araujosetnosy: brett.cannon, carljm, eric.araujo
messages: + msg131706
2011年03月21日 17:48:16carljmsetfiles: + ebe5760afa08.diff
nosy: brett.cannon, carljm, eric.araujo
2011年03月21日 17:47:37carljmsethgrepos: + hgrepo5
messages: + msg131681
nosy: brett.cannon, carljm, eric.araujo
2011年03月21日 00:21:14eric.araujosetnosy: brett.cannon, carljm, eric.araujo
messages: + msg131580
2011年03月20日 20:15:47brett.cannonsetassignee: eric.araujo
messages: + msg131536
nosy: brett.cannon, carljm, eric.araujo
2011年03月18日 09:08:59eric.araujosetnosy: + brett.cannon
messages: + msg131322
2011年03月17日 22:35:09carljmsetmessages: + msg131294
2011年03月17日 21:31:11eric.araujosetnosy: + eric.araujo

messages: + msg131286
versions: + Python 3.3
2011年03月17日 19:47:23carljmsetfiles: + 87df1d37c88e.diff
keywords: + patch
2011年03月17日 19:47:00carljmcreate

AltStyle によって変換されたページ (->オリジナル) /