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

rpm: use %cmake macros #564

Merged
merged 1 commit into from
Nov 29, 2023
Merged

rpm: use %cmake macros #564

merged 1 commit into from
Nov 29, 2023

Conversation

rezib
Copy link
Contributor

@rezib rezib commented Nov 8, 2023

Documentation about these macros is available in Fedora project packaging guidelines:
https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/

One notable advantage of these macros is that they transparently define compiler and linker flags with distribution defaults.

Documentation about these macros is available in Fedora project
packaging guidelines:
https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/

One notable advantage of these macros is that they transparently define
compiler and linker flags with distribution defaults.

Signed-off-by: Rémi Palancher <[email protected]>
@cedeyn
Copy link

cedeyn commented Nov 28, 2023

Seems to properly fix #547

@adammoody adammoody merged commit a80a6da into hpc:main Nov 29, 2023
@adammoody
Copy link
Member

adammoody commented Nov 29, 2023

Thank you, @rezib and @cedeyn .

@adammoody
Copy link
Member

One more addition I see in #547 is that it adds some build-time dependencies.

BuildRequires: cmake, libattr-devel, libarchive-devel, bzip2-devel, lustre-client-devel, openssl-devel

I'm guessing those are needed in clean build environments. Would adding those interfere with your build?

@rezib
Copy link
Contributor Author

rezib commented Nov 29, 2023

I'm guessing those are needed in clean build environments. Would adding those interfere with your build?

Absolutely not! We added these build requirements as well in our in-house spec file actually. We simply didn't contribute this change as we also include other build requirements that may not be relevant in the general case.

Thank you for the merge!

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.

3 participants