Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-31904: Fix site and sysconfig modules for VxWorks RTOS #21821
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
pxinwr commented Aug 11, 2020 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
Lib/test/test_site.py Outdated
| if sys.platform != "vxworks": | ||
| self.assertTrue(user_base.startswith('~' + os.sep), | ||
| user_base) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind to test the expected user_base value? Something like:
self.assertEqual(user_base, sys.prefix) There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
Uh oh!
There was an error while loading. Please reload this page.
| @@ -0,0 +1 @@ | |||
| Fix site and sysconfig modules for VxWorks RTOS | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest:
| Fix site and sysconfig modules for VxWorks RTOS | |
| Fix site and sysconfig modules for VxWorks RTOS which has no home directories. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified as suggested.
c7f2c40 to 2888670Compare
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does os.path.expanduser('~') return on VxWorks?
Instead of changing sysconfig and site, maybe posixpath.expanduser() could get a special case to return sys.prefix?
Lib/sysconfig.py Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind to move it after "if env_base.." and before "def joinuser..." to clarify that the joinuser() function makes no sense on VxWorks?
Same remark for site.py which has copy of this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. Performance is also better with your idea. Modified.
Doc/library/site.rst Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would mind to try to write the different cases as a bullet point for readability?
Default value: * :file:`~/.local/lib/python{X.Y}/site-packages` on UNIX and non-framework macOS builds. * :file:`~/Library/Python/{X.Y}/lib/python/site-packages` on macOS framework builds. * :file:`{%APPDATA%}\\Python\\Python{XY}\\site-packages` on Windows. * :file:`{sys.prefix}/lib/python{X.Y}/site-packages` on VxWorks. This directory is a site directory, (...) Same request for USER_BASE below.
Note: if you update the doc, please replace "Mac OS X" with "macOS" (as I did).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
pxinwr commented Dec 4, 2020 • edited by vstinner
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by vstinner
Uh oh!
There was an error while loading. Please reload this page.
It depends on whether user management feature is enabled or not on VxWorks. By default this feature is disabled. And expanduser() will encounter KeyError then returning the unchanged path. If user management feature is enabled, the pw_dir attribute will be None so that expanduser() will raise exception. So I created PR 23530 to fix this.
Looks sys.prefix is for system-wide settings whereas the return value of |
eccff0c to 795f0e4Compare
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, this change looks reasonable.
The relationship between ENABLE_USER_SITE=False and (USER_SITE=None, USER_BASE=None) may be enhanced to make the code even more generic, but I don't see how it should be done. I'm fine with the current code. Also, sysconfig._getuserbase() doesn't have ENABLE_USER_SITE, so it's ok to use None as a marker "there is no user directory".
vstinner commented Dec 20, 2020
vstinner commented Dec 20, 2020
The second version of the PR is way better, thanks for the update. |
pxinwr commented Dec 21, 2020
@vstinner Thanks for your effort. |
VxWorks RTOS has no home directory for users. Sure it can't recognize the "~" directory. sys.prefix is always set as "/usr" on VxWorks. So for site and sysconfig module, we just simply set the user base as sys.prefix for users. And user site packages path will be sys.prefix/lib/pythonX.Y/site-packages
https://bugs.python.org/issue31904