Skip to content
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

Use CMake USE_SOURCE_PERMISSIONS to install the header files #9778

Closed
wants to merge 1 commit into from

Conversation

dangars
Copy link
Contributor

@dangars dangars commented Mar 30, 2020

Without the option USE_SOURCE_PERMISSIONS, in some corner cases the permissions of the header files can be modified and that can cause problems for the quotas of certain clusters.

In certain clusters the administrator uses the setgid "s" permission to track the quotas. Without the option USE_SOURCE_PERMISSIONS, CMake removes the setgid permission and that can generate the error "Disk quota exceeded".

A description of the option can be found in:
https://cmake.org/cmake/help/v3.8/command/install.html

Without the option USE_SOURCE_PERMISSIONS, in some corner cases the permissions
of the header files can be modified and that can cause problems for the quotas
of certain clusters.

In certain clusters the administrator uses the setgid "s" permission to track
the quotas. Without the option USE_SOURCE_PERMISSIONS, CMake removes the
setgid permission and that can generate the error "Disk quota exceeded".
Copy link
Member

@tamiko tamiko left a comment

Choose a reason for hiding this comment

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

I don't see a problem with enabling this option unconditionally. :-)
(This option had been around since CMake 2.8.11.)

@tamiko
Copy link
Member

tamiko commented Mar 30, 2020

/rebuild

@tjhei
Copy link
Member

tjhei commented Mar 30, 2020

Could it happen with this change that a system-wide installation will have wrong permissions (for example not readable by everyone or writeable by everyone)?

@tamiko
Copy link
Member

tamiko commented Mar 30, 2020

mhm No, file permissions shouldn't be a problem. But this got me start thinking about file ownership (which would indeed be a problem). Let me check first.

@tamiko
Copy link
Member

tamiko commented Mar 30, 2020

@dangars I am curious about one thing - we also install the library and a number of generated files for which USE_SOURCE_PERMISSIONS is unavailable. Do these magically acquire the setgid bit?

@tjhei @dangars I agree with @tjhei - I can not meaningfully assess what this configuration option does. And it might interfere with cmake default configurations that users/distributions might set.
As an alternative, it might be better to have a configure option instead.

@dangars Does setting the CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS work? Or would you consider installing via DESTDIR, fixing up the permissions and copying to final destination to be a viable option?

@dangars
Copy link
Contributor Author

dangars commented Mar 30, 2020

Another option is to allow an external CMake parameter to allow changing the default permissions for "FILE" and "PROGRAMS". This gives a hint:

If USE_SOURCE_PERMISSIONS is specified and FILE_PERMISSIONS is not, file permissions will be copied from the source directory structure. If no permissions are specified files will be given the default permissions specified in the FILES form of the command, and the directories will be given the default permissions specified in the PROGRAMS form of the command.

@dangars
Copy link
Contributor Author

dangars commented Mar 30, 2020

@dangars I am curious about one thing - we also install the library and a number of generated files for which USE_SOURCE_PERMISSIONS is unavailable. Do these magically acquire the setgid bit?

Yes, although I do not fully understand why.

@dangars Does setting the CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS work? Or would you consider installing via DESTDIR, fixing up the permissions and copying to final destination to be a viable option?

I'm going to try this and I'll come back to you in a couple of hours.

@dangars
Copy link
Contributor Author

dangars commented Mar 30, 2020

As an alternative, it might be better to have a configure option instead.

@dangars Does setting the CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS work? Or would you consider installing via DESTDIR, fixing up the permissions and copying to final destination to be a viable option?

I tried the to place the following code in different places, such as the main CMakeLists.txt, include/CMakeLists.txt

set(CMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS
  SETGID
   )

It didn't work out. I do not not why it didn't work out.

DESTDIR does not help because the quota is applied to all of my cluster folders.

My case is a "corner case". I understand that the patch of this PR may have unpredictable consequences in other workflows. I'm happy to keep this as a personal patch that I'll use when I'll install deal.II in my cluster. If you want I can document it, add the patch to the deal.II tree and explain how to apply it:

git apply cmake_permissions.patch

What do you think about this solution? If this is a good option, let me know where to place the patch and where to document it.

@tamiko
Copy link
Member

tamiko commented Mar 30, 2020 via email

@dangars
Copy link
Contributor Author

dangars commented Mar 30, 2020

We could document this in the Wiki - that's probably the most reasonable approach. As a tangential question - you seem to be compiling in a distributed file system already. Have you considered simply not "installing" deal.II at all? You can use any valid (and fully compiled) build directory directly as DEAL_II_DIR.

I didn't know that! Actually that is a good solution :)

I'll document all this including your last solution in the Wiki!

@dangars dangars closed this Mar 30, 2020
@bangerth
Copy link
Member

bangerth commented Mar 30, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants