prevent PermissionError when using venv creator on some systems#2543
prevent PermissionError when using venv creator on some systems#2543gaborbernat merged 5 commits intopypa:mainfrom
Conversation
gaborbernat
left a comment
There was a problem hiding this comment.
Test and changelog too please.
| text = self.instantiate_template(replacements, template, creator) | ||
| dest = to_folder / self.as_name(template) | ||
| # remove the file if it already exists - this prevents permission | ||
| # errors when the dest is not writeable |
There was a problem hiding this comment.
we can only remove if the parent directory is write-able, no? why would this file be not write-able and the parent be so?
There was a problem hiding this comment.
The venv creator specifically uses shutil.copymode on activate* files only (here). Other files in the directory (or the directory itself) are not affected.
> python3.7 -m virtualenv --creator venv venv
PermissionError: [Errno 13] Permission denied: '/..../venv/bin/activate'
> ls -la venv/bin/
drwxr-xr-x 2 user group 16 Apr 14 15:51 .
drwxr-xr-x 5 user group 8 Apr 14 15:51 ..
-r--r--r-- 1 user group 2207 Apr 14 15:51 activate
-r--r--r-- 1 user group 1259 Apr 14 15:51 activate.csh
-r--r--r-- 1 user group 2411 Apr 14 15:51 activate.fish
-rwxr-xr-x 1 user group 243 Apr 14 15:51 pip
-rwxr-xr-x 1 user group 243 Apr 14 15:51 pip-3.7
-rwxr-xr-x 1 user group 243 Apr 14 15:51 pip3
-rwxr-xr-x 1 user group 243 Apr 14 15:51 pip3.7
lrwxrwxrwx 1 user group 9 Apr 14 15:51 python -> python3.7
lrwxrwxrwx 1 user group 9 Apr 14 15:51 python3 -> python3.7
lrwxrwxrwx 1 user group 14 Apr 14 15:51 python3.7 -> /bin/python3.7
-rwxr-xr-x 1 user group 230 Apr 14 15:51 wheel
-rwxr-xr-x 1 user group 230 Apr 14 15:51 wheel-3.7
-rwxr-xr-x 1 user group 230 Apr 14 15:51 wheel3
-rwxr-xr-x 1 user group 230 Apr 14 15:51 wheel3.7
Do you mean test that this issue is no longer present? On our system (where installed files are not writeable by the owner by default) we saw several failures before, and I don't see those now: FAILED tests/unit/create/via_global_ref/test_build_c_ext.py::test_can_build_c_extensions[venv]
FAILED tests/unit/create/test_creator.py::test_create_clear_resets[clear-venv]
FAILED tests/unit/create/test_creator.py::test_prompt_set[magic-venv]
FAILED tests/unit/create/test_creator.py::test_home_path_is_exe_parent[venv]
FAILED tests/unit/create/test_creator.py::test_prompt_set[None-venv]But I will try to think about a test that you might "use" as well. |
|
Can you replicate those failing tests within the CI and add a test for that? Also the pre-commit.ci is failin.g |
@gaborbernat Maybe you can add writeable to the whitelist? |
|
That's up to the PR creator 👍 |
Sure, I can add it. EDIT: it seems that writable is already being used in the codebase and writeable is not, so I changed it. |
gaborbernat
left a comment
There was a problem hiding this comment.
Still needs test that would fail the CI without this fix.
|
I am looking into that. |
|
I added a test that fails on Linux without the rest of this change (inspired by how |
|
Hmmm, one test failed but I don't think that was caused by this change. |
Fixes #2419