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.

Sign up for GitHub

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

Jump to bottom

Add Intel oneAPI and LLVM for Windows Action. #3661

Closed
wants to merge 3 commits into from
Closed

Add Intel oneAPI and LLVM for Windows Action. #3661

wants to merge 3 commits into from

Conversation

hyoklee
Copy link
Member

@hyoklee hyoklee commented Oct 12, 2023 •
edited
Loading

This simple action may help Windows users (e.g., #3275 user).

This action is also good for 1.14.3 release testing as well,
since it uses the latest CMake 3.27.7 and Intel oneAPI for Windows.

It has one failure:

99% tests passed, 1 tests failed out of 2199

Total Test time (real) = 563.81 sec

The following tests FAILED:
	1731 - H5DUMP-tfloatsattrs (Failed)

@hyoklee hyoklee changed the title Add Intel oneAPI/MPI and LLVM for Windows Action. Add Intel oneAPI and LLVM for Windows Action. Oct 12, 2023
@byrnHDF
Copy link
Contributor

byrnHDF commented Oct 12, 2023

Why is this not added to the intel-cmake.yml file?

byrnHDF
byrnHDF requested changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@byrnHDF byrnHDF left a comment

Choose a reason for hiding this comment

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

Integrate into intel-cmake.yml file

@byrnHDF
Copy link
Contributor

byrnHDF commented Oct 12, 2023

Also why is CMake 3.27.7 mentioned - the minimum for hdf5 is 3.18.

@mattjala mattjala added Merge - Develop Only This cannot be merged to previous versions of HDF5 (file format or breaking API changes) Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Component - Build CMake, Autotools Type - Improvement Improvements that don't add a new feature or functionality labels Oct 12, 2023
@hyoklee
Copy link
Member Author

hyoklee commented Oct 12, 2023

Also why is CMake 3.27.7 mentioned - the minimum for hdf5 is 3.18.

Spack is at 3.27.7.

https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/cmake/package.py

@hyoklee
Copy link
Member Author

hyoklee commented Oct 12, 2023 •
edited
Loading

Why is this not added to the intel-cmake.yml file?

I want to have a separate badge. E.g.,

https://github.com/hdfeos/hdf5/blob/develop/WATCHME.md

@byrnHDF
Copy link
Contributor

byrnHDF commented Oct 12, 2023

We will need to do badges differently, Every callable workflow can return a status all the way back up the stack. One solution might be to use a json file.
Another possibility: https://github.com/marketplace/actions/dynamic-badges

Workflows need to be kept consistent.

derobins
derobins reviewed Oct 13, 2023
View reviewed changes
.github/workflows/win-ninja-icx.yml
name: windows ninja icx

on:
push:
Copy link
Member

Choose a reason for hiding this comment

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

Should also be PR

byrnHDF
byrnHDF reviewed Oct 23, 2023
View reviewed changes
tools/test/h5dump/CMakeTests.cmake
@@ -426,7 +426,7 @@
configure_file(${PROJECT_SOURCE_DIR}/exportfiles/tbinregR.exp ${PROJECT_BINARY_DIR}/testfiles/std/tbinregR.exp NEWLINE_STYLE CRLF)
#file (READ ${PROJECT_SOURCE_DIR}/exportfiles/tbinregR.exp TEST_STREAM)
#file (WRITE ${PROJECT_BINARY_DIR}/testfiles/std/tbinregR.exp "${TEST_STREAM}")
HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/expected/tfloatsattrs.wddl" "${PROJECT_BINARY_DIR}/testfiles/std/tfloatsattrs.ddl" "h5dump_std_files")
#HDFTEST_COPY_FILE("${PROJECT_SOURCE_DIR}/expected/tfloatsattrs.wddl" "${PROJECT_BINARY_DIR}/testfiles/std/tfloatsattrs.ddl" "h5dump_std_files")
Copy link
Contributor

Choose a reason for hiding this comment

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

This change should not be changed in an action - this would need to be a different PR.

byrnHDF
byrnHDF requested changes Oct 23, 2023
View reviewed changes
Copy link
Contributor

@byrnHDF byrnHDF left a comment

Choose a reason for hiding this comment

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

This is not a solution

@byrnHDF
Copy link
Contributor

byrnHDF commented Nov 10, 2023

Resolved by #3836

@byrnHDF byrnHDF closed this Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derobins derobins derobins left review comments

@byrnHDF byrnHDF byrnHDF requested changes

@lrknox lrknox Awaiting requested review from lrknox lrknox is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@jhendersonHDF jhendersonHDF Awaiting requested review from jhendersonHDF jhendersonHDF is a code owner

@qkoziol qkoziol Awaiting requested review from qkoziol qkoziol is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@bmribler bmribler Awaiting requested review from bmribler bmribler is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@mattjala mattjala Awaiting requested review from mattjala mattjala is a code owner

@brtnfld brtnfld Awaiting requested review from brtnfld brtnfld is a code owner

Assignees
No one assigned
Labels
Component - Build CMake, Autotools Merge - Develop Only This cannot be merged to previous versions of HDF5 (file format or breaking API changes) Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Improvement Improvements that don't add a new feature or functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hyoklee @byrnHDF @derobins @mattjala