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

Simplify CMake installation in Dockerfiles #222

Merged
merged 7 commits into from
Jan 27, 2025
Merged

Conversation

yasahi-hpc
Copy link
Collaborator

Fixes CI failures due to the build failure of clang Dockerfile. (See build failure)

@yasahi-hpc yasahi-hpc self-assigned this Jan 22, 2025
@yasahi-hpc yasahi-hpc added bug Something isn't working CI cleanup labels Jan 22, 2025
@yasahi-hpc yasahi-hpc requested a review from pzehner January 22, 2025 13:43
@pzehner
Copy link
Collaborator

pzehner commented Jan 22, 2025

@yasahi-hpc Please propagate the Clang Dockerfile modification to the other ones.

@yasahi-hpc
Copy link
Collaborator Author

@yasahi-hpc Please propagate the Clang Dockerfile modification to the other ones.

Thanks!
Can we use the same key for different CMake version? For intel, we use CMake 3.25

@pzehner
Copy link
Collaborator

pzehner commented Jan 22, 2025

Shouldn't we use the same CMake version everywhere?

By the way, I should document how to get this damn key.

@yasahi-hpc
Copy link
Collaborator Author

That is nice!

Actually, oneAPI says they recommend CMake 3.25 as minimum.

@pzehner
Copy link
Collaborator

pzehner commented Jan 22, 2025

I added some documentation. Could you use it and check if it's understandable?

@yasahi-hpc
Copy link
Collaborator Author

I added some documentation. Could you use it and check if it's understandable?

Thanks. It is good. I just added an URL of CMake 3.23.2 as an example

@yasahi-hpc yasahi-hpc changed the title simplify Dockerfile for clang CI Simplify CMake installation in Dockerfiles Jan 22, 2025
@yasahi-hpc
Copy link
Collaborator Author

@pzehner May I have another review please?
I am also interested in merging two md files into one.

pzehner
pzehner previously approved these changes Jan 23, 2025
Copy link
Collaborator

@pzehner pzehner left a comment

Choose a reason for hiding this comment

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

I'm good with it.

@pzehner
Copy link
Collaborator

pzehner commented Jan 23, 2025

I differentiated the two document files because one is intended for maintenance. But if you want to merge the two files into one, feel free.

@yasahi-hpc
Copy link
Collaborator Author

@pzehner Thank you for your review. Let me merge files.

@yasahi-hpc
Copy link
Collaborator Author

I should have added [ci skip]. In the CI, we encountered the error related to the permission. Can I fix the permission issue in this PR? @pzehner

@yasahi-hpc
Copy link
Collaborator Author

Seems OK now. I will merge this. Thank you for your help @pzehner

@yasahi-hpc yasahi-hpc merged commit ccdedaa into main Jan 27, 2025
25 checks passed
@yasahi-hpc yasahi-hpc deleted the fix-clang-container branch January 27, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI cleanup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants