Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 20, 2020
Merged

bpo-31904: Fix site and sysconfig modules for VxWorks RTOS #21821

merged 1 commit into from
Dec 20, 2020

Conversation

pxinwr
Copy link
Contributor

@pxinwr pxinwr commented Aug 11, 2020 •
edited by bedevere-bot
Loading

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

vstinner
vstinner reviewed Nov 28, 2020
View reviewed changes
Lib/test/test_site.py Outdated

if sys.platform != "vxworks":
self.assertTrue(user_base.startswith('~' + os.sep),
user_base)
Copy link
Member

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

Misc/NEWS.d/next/Library/2020-08-11-17-44-07.bpo-31904.cb13ea.rst Outdated
@@ -0,0 +1 @@
Fix site and sysconfig modules for VxWorks RTOS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest:

Suggested change
Fix site and sysconfig modules for VxWorks RTOS
Fix site and sysconfig modules for VxWorks RTOS which has no home directories.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified as suggested.

@pxinwr pxinwr force-pushed the fix-issue-31904-site branch from c7f2c40 to 2888670 Compare December 2, 2020 06:45
vstinner
vstinner reviewed Dec 2, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a 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
@@ -201,6 +201,10 @@ def joinuser(*args):
return joinuser("~", "Library", sys._framework,
"%d.%d" % sys.version_info[:2])

# VxWorks has no home directories
if sys.platform == "vxworks":
return sys.prefix
Copy link
Member

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.

Copy link
Contributor Author

@pxinwr pxinwr Dec 4, 2020 •
edited
Loading

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
on Windows. This directory is a site directory, which means that
framework builds, :file:`{sys.prefix}/lib/python{X.Y}/site-packages` for
VxWorks, and :file:`{%APPDATA%}\\Python\\Python{XY}\\site-packages` on
Windows. This directory is a site directory, which means that
Copy link
Member

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@pxinwr
Copy link
Contributor Author

pxinwr commented Dec 4, 2020 •
edited by vstinner
Loading

What does os.path.expanduser('~') return on VxWorks?

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.

Instead of changing sysconfig and site, maybe posixpath.expanduser() could get a special case to return sys.prefix?

Looks sys.prefix is for system-wide settings whereas the return value of posixpath.expanduser() is used for user specific settings. If we return sys.prefix from posixpath.expanduser(), any chance things get mixed up to mess? Now I get stuck on which one, None or sys.prefix should be returned for user base and user site. What is your idea? If determined to None, ENABLE_USER_SITE should also be set to False. Since sys.prefix has been added to sys.path, so setting user base to None has no impact on sys.path.

@pxinwr pxinwr force-pushed the fix-issue-31904-site branch from eccff0c to 795f0e4 Compare December 7, 2020 10:49
vstinner
vstinner approved these changes Dec 20, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a 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
Copy link
Member

posixpath.expanduser() has been modified on VxWorks by PR #23530 (commit 75dabfe): return the path unmodified if pwd.getpwuid().pw_dir is None. This change doesn't help site/sysconfig, so this PR remains useful/relevant.

@vstinner
Copy link
Member

The second version of the PR is way better, thanks for the update.

@vstinner vstinner merged commit ab74c01 into python:master Dec 20, 2020
@pxinwr
Copy link
Contributor Author

pxinwr commented Dec 21, 2020

@vstinner Thanks for your effort.

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@pxinwr @adorilson
bpo-31904: Fix site and sysconfig modules for VxWorks RTOS (pythonGH-…
1bda4fa 
…21821)
@pxinwr pxinwr deleted the fix-issue-31904-site branch May 7, 2021 07:42
@kuhlenough kuhlenough mannequin mentioned this pull request Jan 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pxinwr @vstinner @the-knights-who-say-ni @bedevere-bot