-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
openmpi: fix build on darwin #332983
Closed
Closed
openmpi: fix build on darwin #332983
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
c6fea66
openmpi: remove pmix reference from comment string
markuskowa 405a739
openmpi: fix pmix config parameters
markuskowa 2149aef
openmpi: apply nixfmt
markuskowa f902f42
openmpi: run postInstall only on Linux
markuskowa c5a289b
openmpi: apply nixfmt
markuskowa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'd prefer to only condition the substitution and not all of the postInstall... We might enable multiple outputs in the future for other platforms, and you'd still want to delete all
*.la
files...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.
Then we need extra condition for the split output moves too. If we want to enable multiple outputs in the future for other platforms we can make it more granular later? I do not know what darwin does with .la files. I know that they are not needed on Linux.
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.
No you won't. That was the purpose of 2a4636b .
The diff will be much smaller thanks to 2a4636b .
Yea that line was peculiar to me so I didn't touch it.. I suspect they might be generated on Darwin as well, so I wouldn't touch it even now. For sure it doesn't hurt if these files are not found at all.