-
Notifications
You must be signed in to change notification settings - Fork 45
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
Update TriBITS install targets to set permissions and owning group #314
Comments
@bartlettroscoe, do you have an idea on time-to-completion for this? I'm thinking I need to hack in a temporary solution for our deploys on chama/eclipse/stria. For some reason things work just fine on mutrino/vortex though. |
@jmgate, I am going to try to get this done today and submit a Trilinos PR for you to review (testing should not be needed on your end). |
Some useful info from Kitware developers to keep in mind while I implement this. From: Bartlett, Roscoe A Hello Brad, Thanks for pointing that out the order specification. Since this will be called at the base-level CMakeLists.txt scope, I will not need to worry about ordering between directories. That means I just need to call: Install(SCRIPT "<...>/set_installed_group_and_permissions.sh) at the very end of the TRIBITS_PROJECT() command, after everything else might have been installed. (And projects will need to be careful to not do any extra installs after the TRIBITS_PROJECT() command ends but instead any extra rep-level or project-level installs in the files Thanks! -Ross From: Brad King brad.king@kitware.com On 4/23/2020 4:47 PM, Bartlett, Roscoe A wrote:
Yes, scripts will stay ordered. The overall introduction at the top of that page: https://cmake.org/cmake/help/v3.17/command/install.html#introduction says "Rules ... within a source directory are executed in order during installation". It also says the "order across directories is not defined", but if CMP0082: https://cmake.org/cmake/help/v3.17/policy/CMP0082.html is set to NEW then I think the order is now defined across directories too. -Brad |
How this has all of the parts needed to ensure that the group and directory permissions are set correctly on install that matches the ATDM Trilinos case reproted in ATDV-241.
How this has all of the parts needed to ensure that the group and directory permissions are set correctly on install that matches the ATDM Trilinos case reproted in ATDV-241.
I created the PR for this #315. It looks like using:
to try to get it to run after all of the other files get installed does not actually work. If you look at the TriBITS test for the 'install' command at: you will see that the script gets called pretty early on during the 'install' command showing:
It seems like everything in subdirs is installed after the project-level stuff, even though that Is this how it is supposed to work? |
I also fixed the group read-only permissions. Not sure how I missed that. Note that we can now set permissions correctly for all versions of CMake!
…riBITSPub#314) I also removed the surrounding whitespace around the tests.
I found a workaround that I show in commit 7790c7e. The workaround is to add the Note that the most current documentation at: which says:
is not true if policy: is set. You should update that documentation to mention policy CMP0082 (since what is currently stated "The order across directories is not defined" is actually deprecated behavior). However, we can't rely on that policy because the plan in #299 it to make TriBITS require CMake 3.13+ but this was only added in CMake 3.14. (But we are setting that policy to NEW when defined just to be safe.) So now I just need to update documentation, clean up the commits, and this is ready to merge to 'master' and update to Trilinos. |
…y_package test (TriBITSPub#314) Now I know that if a package fails to build and install that the other packages that did build will install and have the correct permissions.
CMake MR 4672 updates the |
…TSPub#314) I also fixed numerious mispelling of "UNITEST" to "UNITTEST".
…SPub#314) Refectored some other installation-cleanup code to by extracting some functions.
… CMAKE_INSTALL_PREFIX (TriBITSPub#314)
…riBITSPub#314, ATDV-241) This is needed to fix some permisisons problems related to that ATDM Trilinos builds (see ATDV-241). See the updated documentation and unit tests for details. Other things I did as part of this: * Options ${PROJECT_NAME}_MAKE_INSTALL_GROUP_READABLE and ${PROJECT_NAME}_MAKE_INSTALL_WORLD_READABLE no longer conditional (since they are enforced by recursive chmod). * Set policy CMP0082 to make install() commands run in order based on how they were executed in the CMake configure. * Added TRIBITS_DIR_IS_BASEDIR() and unit tests * Fixed numerious mispelling of "UNITEST" to "UNITTEST". * Refectored some other installation-cleanup code to by extracting some functions.
FYI: PR #315 is merged. Now to snapshot this to Trilinos and update the ATDM Trilinos configuration to automatically fixup the permissions for all ATDM Trilinos builds. Putting this issue in review. |
Shoot, one issue that I overlooked is what happens when to different user accounts try to install into the same base directory. For that ATDM Trilinos builds, they install (as of today) under the same
So for today it was:
Looking at that directory and its subdirs today on the SNL HPC machines, we can see what happened:
We can see that the 'atdm-devops-admin' account completed the install first and created the directory:
first and therefore all of the 'jenkins' jobs trying to install for the envs 'van1-tx2' (on 'stria') and 'ats2' (on 'vortex') have the commands:
failing since 'jenkins' does not own that directory. You can see this in the
Now, this did not cause the install to fail in anyway because the driver script
And we see the right group and permissions set on the actual CMAKE_INSTALL_PREFIX dir above. But we can't be having build errors showing up on CDash for this. Therefore, I need to add logic to make sure that the owner of these base directories are the same as the owner trying to change the group and permissions. If the owner is not the same, then just print a note that the group and permissions can not be changed because the owner is different. I can implement this and test it manually, but it will be hard to write an automated test for this. But it did occur to me that I could use the 'atdm-devops-admin' account and the 'run-as-atdm-devops-admin' utility executable to set up a test that can simulate this in TriBITS. I will do that. |
The test took about 10x more time and care to write than the actual production code :-(
The test took about 10x more time and care to write the tests than the actual production code :-(
The test took about 10x more time and care to write the tests than the actual production code :-(
…nstall-base-dir (ATDV-241) Avoids trying to change the group or permissions on a base install dir you don't own. (See TriBITSPub/TriBITS#314)
With the pushed merge commit be318a4 to TriBITS 'master' and the Trilinos TriBITS snapshot PR trilinos/Trilinos#7297, this should not be 100% complete. Moving back to "in review" to watch this some more. |
@jmgate verified this fixed the install permissions problems with EMPIRE. (See https://cee-gitlab.sandia.gov/EM-Plasma/BuildScripts/issues/301#note_415312) |
…Windows (TriBITSPub#314) This replaces logic that just ignored the instal perm/group modification options for Windows platforms added as part of Story TriBITSPub#314. Now if the user tries to set any of these options, it errors out with a (hopefully) good error message. This was flagged as part of the review of PR TriBITSPub#327.
…trilinos/Trilinos#7881) Build/Test Cases Summary Enabled Packages: Enabled all Packages 0) MPI_DEBUG => passed: passed=382,notpassed=0 (2.25 min) 1) SERIAL_RELEASE => passed: passed=382,notpassed=0 (1.43 min) 2) MPI_DEBUG_CMake-3.17.0 => passed: passed=387,notpassed=0 (2.85 min) 3) SERIAL_RELEASE_CMake-3.17.0 => passed: passed=387,notpassed=0 (2.40 min) Other local commits for this build/test group: f4001a1, 1ed3811, e8467f7
…#314, TriBITSPub#363) With CMake 3.21, with umask=g-rwx,o-rwx you need explicitly set g+rX for the group permissions to be r-X when setting <Project>_MAKE_INSTALL_WORLD_READABLE=ON which was doing just: chmod -R o+rX <dir> Now, when you ony set <Project>_MAKE_INSTALL_WORLD_READABLE=ON, you get: chmod -R g+rX,o+rX <dir> For some reason, with older verisons of CMake, it was setting the default group permission as 'r-x' when it should not have been. I ran exactly the same build on the same machine (crf450) with the same env except I used CMake 3.21.0 vs. 3.17.1 and I had to make this change to get the same permissions on the base install directory. Strange.
Description
This story is to extend the both (if possible) the built-in
install
target as well as the TriBITS custominstall_package_by_package
target to set the correct owning group and the correct group and other permissions after the install of all of the files that are not being set correctly in in some cases currently (see ATDV-241). This extends the existing support for the TriBITS variables:which impact existing installed files and newly created directories. The problem is that current implementation does not result in installed files and directories in certain cases (again, see see ATDV-241).
In some cases the correct owning group is not being set correctly because some systems don't allow implementation of the group sticky bit (i.e. were one runs
chgrp -R <owning-group> <install-base> ; chmod -R g+s <install-base>
). Therefore, the owning group on the files and directories that CMake installs cannot be controlled in that way and instead the desired owning group must be manually set after the fact. This occurs, for example, in the case where a shared entity account drives the install but where the default group for that account is not the desired owning group of the installed files and directories. (This is the case where the 'jenkins' entity account with default group 'jenkins' installs Trilinos but where thewg-run-as-admin-devops
group needs to own the install after it is complete.)One solution to the group ownership problem is to this is to allow the user to select the desired owning group of the created installed files and directories at configure time by setting the TriBITS cache var:
and then adding the custom install commands to manually set the owning group to
<owning-group>
after the install is complete.Also, the correct group and other permissions can't be controlled by initially setting the
umask
when one is having to recursively install a set of files and directories using:One must use
USE_SOURCE_PERMISSIONS
to preserve execute permissions on various scripts when they get installed. The reason that we have to useUSE_SOURCE_PERMISSIONS
is that we don't always know a-priori which of the files in<src-dir>
have execute permissions and therefore we can't list them out to set their execute permissions (or we could just list them out explicitly). (This was needed to install the custom system settings directory in the ATDM Trilinos configuration where that directory can contain arbitrarily named shell scripts liketrilinos_jsrun
.) However, usingUSE_SOURCE_PERMISSIONS
overrides the other permission options that CMake would otherwise use based on the specific permissions options being passed into otherinstall()
commands and usingCMAKE_INSTALL_DEFAULT_DIRECTORY_PERMISSIONS
as described in Setting install directory permissions. So we need a way to fix those permissions after the fact. (Kitware contractor suggested fixing them manually with a recursivechmod
.) In this case, we will need to fix the group and other read, write, and execute permissions appropriately. This may include needing to set group write access by setting:Note that the CMake install command may create base directories above
${CMAKE_INSTALL_PREFIX}
if it needs to create them so we need to know where to start fixing the group ownership and permissions. To make this explicit, we can ask the user to set the cache var:where
<install-base-dir>
is a base directory of of${CMAKE_INSTALL_PREFIX}
that may not exist before the install is done and therefore the install command may create it. (If<Project>_SET_GROUP_AND_PERMISSIONS_ON_CMAKE_INSTALL_PREFIX_BASE
is not set, then no fix up of permissions will be performed.)Therefore, to configure to properly set the owning group and permissions, one would configure with the options:
In the ATDM Trilinos configuration, this would be, for example:
which gets run by the 'jenkins' entity account. That would ensure that all of the files and directories from
/projects/atdm_devops/trilinos_installs/2020-04-10
are owned by thewg-run-as-atdm-devops
group and have the the directory and executable file permissionsrwxrwxr-x
and other regular files will have permissionsrw-rw-r--
.Definition of Done
Try to have both the built-in
install
and the custominstall_package_by_package
targets set the correct owning group and permissions for all installed files and directories (for whatever files get installed).Document the new group and permissions handling in the TriBITS Project Build Reference (replacing the current "Setting install directory permissions") to describe these new options and behaviors and why they are needed (i.e. systems were the group sticky bit is turned off and where
umask
can't be used due toUSE_SOURCE_PERMISSIONS
)Tasks
[ ] ???
The text was updated successfully, but these errors were encountered: