Resolve configdir so that it's not a symlink when is_dir() is called #28818
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
|
Quick description of the patch: If you look at this code, only one of the three code paths here resolves their paths before assigning to matplotlib/lib/matplotlib/__init__.py Lines 521 to 528 in 42b88d0 All three of those paths should resolve before assigning, so no symlinks are left over matplotlib/lib/matplotlib/__init__.py Lines 534 to 535 in 42b88d0 |
There are some use cases where a user might have a symlinked home directory (e.g. a corporate home directory may be symlinked to a disk with limited space, and is only accessible to other users via a real path to the underlying disk). Resolving configdir before any mkdir or access checks should avoid this problem. Co-authored-by: Greg Lucas <greg.m.lucas@gmail.com>
89d18d8 to
a8ce9b5
Compare
|
Ok, I realized my thinking about the original problem was off (see #28817 (comment)), but the solution of forcing a In commit 8837d9a, I add a missing error check, and then print two different warnings depending on what actually failed. This should help narrow down what is going on instead of e.g. seeing "path is not a writable directory" when in reality the path was inaccessible or the disk was full. |
For example, a user could have no more disk space, causing mkdir to fail, but not realize it because the error is silently discarded, and the subsequent warning would not indicate the actual issue.
|
Thank you @hintron and congratulations on your first merged Matplotlib PR 🎉 I hope we hear from you agian. |
…mlink when is_dir() is called
|
@tacaswell Shouldn't commit 1b08207 show me as the author and you as the committer? Or does the matplotlib project not do it that way? |
|
Oh, nevermind, that's just the merge commit, not a squashed rebase commit. Carry on :) |
…818-on-v3.9.x Backport PR #28818 on branch v3.9.x (Resolve configdir so that it's not a symlink when is_dir() is called)
Closes #28817.
There are some use cases where a user might have to make ~/.config a symlink (e.g. if a corporate home directory has limited space, and symlinks are used to offload the storage to a separate storage disk). So resolving configdir beforehand should prevent the is_dir() check from failing, causing a warning to be issued and forcing a temporary directory to be used.
PR summary
PR checklist