Skip to content
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

fix support for python 3.13 #13205

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eli-schwartz
Copy link
Member

minstall: fix symlink handling on python 3.13

We passed a wrapper hack for shutil.chown because some functionality wasn't available in the stdlib. It was added in python 3.13 beta1, so the tests fail when we actually test symlink handling.

Fixes failure to run test_install_subdir_symlinks_with_default_umask_and_mode on python 3.13.

...

There's still another failing test that I haven't debugged yet but I am running out of time this Friday.

@@ -161,8 +161,11 @@ def chown(path: T.Union[int, str, 'os.PathLike[str]', bytes, 'os.PathLike[bytes]
real_os_chown(path, uid, gid, dir_fd=dir_fd, follow_symlinks=follow_symlinks)

try:
os.chown = chown
shutil.chown(path, user, group)
if sys.version_info >= (3, 13):
Copy link
Member

Choose a reason for hiding this comment

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

Since you already are in a try block, why not simply try the new syntax, and except the AttributeError for the fallback version?

Copy link
Member Author

Choose a reason for hiding this comment

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

You raise a good point, but perhaps not the one you meant.

abc9eb0

Copy link
Member Author

Choose a reason for hiding this comment

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

try/except is impenetrable to tools that try to upgrade your code when you bump the minimum required python version, and you end up needing TODO comments anyway instead of self-documenting code. It's not even obvious to me that try/except is cleaner.

We passed a wrapper hack for shutil.chown because some functionality
wasn't available in the stdlib. It was added in python 3.13 beta1, so
the tests fail when we actually test symlink handling.

Fixes failure to run test_install_subdir_symlinks_with_default_umask_and_mode
on python 3.13.
…te locals()

"PEP 667: Consistent views of namespaces" caused locals() to be
inconsistent between uses since it is now created afresh every time you
invoke it and writes to it are dropped. `sys._getframe().f_locals` is
equivalent but preserves writes (it doesn't create a new dict) and
unfortunately doesn't help at all as it's documented to be a private
implementation detail of CPython that "should be used for internal and
specialized purposes only".

Work around this by saving locals to a variable reference and both
passing it into runctx and reusing it in lookups of the result. This
works okay for both new and older versions of python.

Per the documentation for locals():

> The contents of this dictionary should not be modified; changes may
> not affect the values of local and free variables used by the
> interpreter.

So... lesson learned? :) This was introduced in commit
c34ee37; before that, we still used
locals() but only to pass local variables *in*.

Bug: python/cpython#115153
@eli-schwartz
Copy link
Member Author

Pylint failure is puzzling. pylint-dev/pylint#9622

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants