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

Fix install for MacOSX (trilinos/Trilinos#7881) #327

Conversation

bartlettroscoe
Copy link
Member

Should fix install on MacOSX systems (see trilinos/Trilinos#7881)

See the commits for more details

How was this tested?

I only run the updated unit tests. I don't have access to a Mac right now to test this myself.

I have not been able to test that installation with this group and/or perms
modification code works on MacOSX but it was reported in
trilinos/Trilinos#7881 that calling 'stat -f %U <file>' works correctly.  (I
hope to be able to test this later as part of general MacOSX testing for
TriBITS.)
…/Trilinos#7881)

Previously, the command 'stat -c %U <path>' was always being run and that
command failed on MacOSX (that uses 'stat -f $U <path>' instead as fixed in
the previous commit).  The issue is that such commands should not even be run
unless the user sets one of the group and/or permissions fixup options that
TriBITS now supports.

With this commit, when the user has not selected any of the group and/or
permissions fixup options, then TriBITS will not exectute any extra code
related to this at all durning configure, build, or install time.

A test was updated to check that the file
set_installed_group_and_permissions.cmake is not even generated in the build
dir if none of the group/perms options are set.  If you change the production
code to always generate the file set_installed_group_and_permissions.cmake,
then the test will fail (which I verified in TDD process).  If that file does
not get generated then it can't be called durring 'install' and therefore
can't call any extra (less portable) comamnds.
@bartlettroscoe
Copy link
Member Author

CC: @ndellingwood

@jmgate, do you have time to review this TriBITS PR before I merge and then snapshot to a Trilinos branch to merge to Trilinos 'develop'?

If we are going to be doing a real review, the that should occur in a PR in the TriBITS repo and not in snapshots to the Trilinos repo (that strips out all of the TriBITS tests).

@jmgate
Copy link
Collaborator

jmgate commented Sep 17, 2020

Sorry I missed this, @bartlettroscoe. Apparently I need to adjust my GitHub notification settings.

@jmgate
Copy link
Collaborator

jmgate commented Sep 17, 2020

Unfortunately I won't have time to review this until next week, but I have it open now, so I'll get to it early on Monday.

@bartlettroscoe
Copy link
Member Author

Unfortunately I won't have time to review this until next week, but I have it open now, so I'll get to it early on Monday.

@jmgate, that is okay. GitHub allows you to review a PR even after it has been merged. Then I can address any issues post-merge in a follow-up PR.

@bartlettroscoe bartlettroscoe merged commit 1b00a3c into TriBITSPub:master Sep 19, 2020
bartlettroscoe added a commit to trilinos/Trilinos that referenced this pull request Sep 19, 2020
Origin repo remote tracking branch: 'github/master'
Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git'

At commit:

commit cee1980d053ec2c26301d2389c8b0a677fa262fe
Author:  Roscoe A. Bartlett <[email protected]>
Date:    Sat Sep 19 07:40:49 2020 -0600
Summary: Merge remote-tracking branch 'rab-github/tril-7112-run-serial' (#7112)

This represents the changes in the TriBITS PRs:

* TriBITSPub/TriBITS#327 : Fix install for MacOSX (#7881)

* TriBITSPub/TriBITS#328 : Add support for
  <fullTestName>_SET_RUN_SERIAL=[ON|OFF] (#7112)
Copy link
Collaborator

@jmgate jmgate left a comment

Choose a reason for hiding this comment

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

This looks good to me, @bartlettroscoe. Thanks.

Copy link
Collaborator

@e10harvey e10harvey left a comment

Choose a reason for hiding this comment

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

In general, everything looks great. Please see suggestions. Thanks, Ross!

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this pull request Sep 22, 2020
…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.
bartlettroscoe added a commit that referenced this pull request Sep 22, 2020
…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
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this pull request May 6, 2021
This brings over the fixes to TriBITS in PR TriBITSPub/TriBITS#327 along with
other changes/fixes to these files.

It would have been much hader to try to bring over only the changes to fix the
Mac OSX problem since the version of TriBITS in the Trilinos 13.0 branch is
from almost 1 year ago.

The new changes should avoid calling any of this code by default due to
changes pulled over from PR TriBITSPub/TriBITS#327 as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants