Skip symlinked files when fixing permissions #1439
Conversation
|
Automatic reply from Jenkins: Can I test this? |
| @@ -700,8 +700,15 @@ def adjust_permissions(name, permissionBits, add=True, onlyfiles=False, onlydirs | |||
| for root, dirs, files in os.walk(name): | |||
| paths = [] | |||
| if not onlydirs: | |||
| paths += files | |||
| if not skip_symlinks: | |||
| paths += files | |||
There was a problem hiding this comment.
let's switch to paths.extend(files) here (matches better with the .append used below)
add test for adjust_permissions
| @@ -700,9 +700,17 @@ def adjust_permissions(name, permissionBits, add=True, onlyfiles=False, onlydirs | |||
| for root, dirs, files in os.walk(name): | |||
| paths = [] | |||
| if not onlydirs: | |||
| paths += files | |||
| if not skip_symlinks: | |||
There was a problem hiding this comment.
hmm, maybe it's better to avoid 'double negation' (sort of):
if skip_symlinks:
...
else:
# ...
paths.extend(dirs)There was a problem hiding this comment.
Makes some sense. But after switching this, we have yet another negation inside the for loop -- which has now moved up. Though I would suggest to keep it that way to have the part where something has to be done first.
There was a problem hiding this comment.
Thinking about it, it may actually make sense to switch that too (If we are skipping symlinks and if it is a link, issue a debug message; otherwise do it)
|
Jenkins: ok to test |
|
EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2239/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2240/console for more details). This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
Going in, thanks @geimer! |
Skip symlinked files when fixing permissions
Either the symlinks will point to other files in the installation prefix, so permissions for those files are being handled twice in that case, or the symlinks point to somewhere else (like for PDT), in which case we shouldn't touch the files they point to at all (not only to avoid errors, it's just plain wrong to do so)...
The old behavior is still available via
skip_symlinks=False.