Skip to content

Add read permission to all installed files for everybody#1731

Merged
boegel merged 5 commits intoeasybuilders:developfrom
wpoely86:addreadpermission
May 2, 2016
Merged

Add read permission to all installed files for everybody#1731
boegel merged 5 commits intoeasybuilders:developfrom
wpoely86:addreadpermission

Conversation

@wpoely86
Copy link
Copy Markdown
Member

@wpoely86 wpoely86 commented Apr 21, 2016

If nothing is specified, all installed files will have read permissions
for everybody.

edit: we ran into this with VarScan, where the .jar was only readable for the install user

If nothing is specified, all installed files will have read permissions
for everybody.
Comment thread easybuild/framework/easyblock.py Outdated
perms = stat.S_IWGRP | stat.S_IWOTH
adjust_permissions(self.installdir, perms, add=False, recursive=True, relative=True, ignore_errors=True)
self.log.info("Successfully removed write permissions recursively for group/other on install dir.")
# add read permissions for everybody on all files
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

more like: make sure read permissions for everyone are in place, because usually they're already there...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that boils down to exactly the same?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah, fine :)

@boegel boegel added this to the v2.8.0 milestone Apr 21, 2016
@boegel
Copy link
Copy Markdown
Member

boegel commented Apr 21, 2016

Should be fine imho, because that's what you'd usually expect.
It will make the installation a bit slower for large installations though, since this implies stat'ing all files (again)...

@pescobar, @fgeorgatos, @ocaisa, @rjeschmi: thoughts?

@rjeschmi
Copy link
Copy Markdown
Contributor

Sounds good to me. I wonder what a system would look like that restricted permissions. You'd probably have to do a lot more throughout the module hierarchy to make it work usefully.

@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2949/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2950/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@boegel
Copy link
Copy Markdown
Member

boegel commented May 1, 2016

@wpoely86 broken tests are fixed with wpoely86#20

@boegel boegel changed the title Add Read permission to all installed files for everybody Add read permission to all installed files for everybody May 1, 2016
take into account group setting & fix interpretation of umask
@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2998/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.

@boegel
Copy link
Copy Markdown
Member

boegel commented May 2, 2016

Going in, thanks @wpoely86!

@boegel boegel closed this May 2, 2016
@boegel boegel reopened this May 2, 2016
@boegel boegel merged commit 78dda76 into easybuilders:develop May 2, 2016
@hpcugentbot
Copy link
Copy Markdown

EasyBuild framework unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-framework-pr-builder/2999/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@boegel
Copy link
Copy Markdown
Member

boegel commented May 2, 2016

oops, didn't mean to close, finger slipped... :)

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.

4 participants