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

Skip symlinks in _make_tree_writable() #14227

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DaanDeMeyer
Copy link
Contributor

Trying to chmod a symlink results in trying to chmod the file or directory it points to, not the symlink itself which has no permissions. Either a symlink points to within the tree we're making writable in which case it'll be handled eventually by os.walk() or it points outside of the tree we're making writable in which case we don't want to touch it. Let's avoid touching files outside of the tree by simply skipping symlinks in _make_tree_writable().

Trying to chmod a symlink results in trying to chmod the file or
directory it points to, not the symlink itself which has no
permissions. Either a symlink points to within the tree we're making
writable in which case it'll be handled eventually by os.walk() or
it points outside of the tree we're making writable in which case
we don't want to touch it. Let's avoid touching files outside of the
tree by simply skipping symlinks in _make_tree_writable().
@DaanDeMeyer DaanDeMeyer requested a review from jpakkane as a code owner February 6, 2025 11:32
@@ -1902,7 +1902,7 @@ def _make_tree_writable(topdir: T.Union[str, Path]) -> None:
os.chmod(d, os.stat(d).st_mode | stat.S_IWRITE | stat.S_IREAD)
for fname in files:
fpath = os.path.join(d, fname)
if os.path.isfile(fpath):
if not os.path.islink(fpath) and os.path.isfile(fpath):
Copy link
Member

Choose a reason for hiding this comment

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

Why don't you simply add follow_symlinks=False to chmod ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like I said, symlinks don't have permissions on posix, trying to change them is just weird so we shouldn't try to do it in the first place.

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.

2 participants