-
Notifications
You must be signed in to change notification settings - Fork 218
only actually change permissions using os.chmod in adjust_permissions if the current permissions are not correct already #3125
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
only actually change permissions using os.chmod in adjust_permissions if the current permissions are not correct already #3125
Conversation
… if the current permissions are not correct already
| :param relative: add/remove permissions relative to current permissions (if False, hard set specified permissions) | ||
| :param ignore_errors: ignore errors that occur when changing permissions | ||
| (up to a maximum ratio specified by --max-fail-ratio-adjust-permissions configuration option) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing :param skip_symlinks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That one is deprecated (and if you try using it in EasyBuild v4.x you'll got an error), so I excluded that one on purpose.
|
|
||
| if add: | ||
| os.chmod(path, perms | permissionBits) | ||
| _log.debug("Adding permissions for %s: %s", path, oct(permission_bits)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move this below new_perms and print both the permission_bits and the final result.
Same for removing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then I need another if add: to discriminate below adding/removing of the permission_bits?
The log messages now mention everything: original permissions, added/removed permissions, changed permissions. For (fictional) example:
== 2019-12-12 21:51:39,936 filetools.py:1189 DEBUG Current permissions for /Users/kehoste/software/bzip2/1.0.6/bin/bzip2: 0100777
== 2019-12-12 21:51:39,936 filetools.py:1197 DEBUG Removing permissions for /Users/kehoste/software/bzip2/1.0.6/bin/bzip2: 022
== 2019-12-12 21:51:39,936 filetools.py:1207 DEBUG Changing permissions for /Users/kehoste/software/bzip2/1.0.6/bin/bzip2 to 0100755
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I meant moving it one line down after the new_perms = ... line.
I didn't think of the log below with the actual chmod
|
|
||
| if add: | ||
| os.chmod(path, perms | permissionBits) | ||
| _log.debug("Adding permissions for %s: %s", path, oct(permission_bits)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I meant moving it one line down after the new_perms = ... line.
I didn't think of the log below with the actual chmod
|
Going in, thanks @boegel! |
Using
eb --skipon an installation that was originally performed by someone else currently fails in the permissions step, even if both account are a member of the same group and--group-writable-installdiris used.This change/optimization to
adjust_permissionsfixes that problem...cc @verdurin, @migueldiascosta