Conversation
scheibelp
left a comment
There was a problem hiding this comment.
Let me know if any of these requests seem to miss the point.
|
@jpopelar can you push an empty commit here (see also: #31589 (comment)). |
|
Just pushed up a commit to trim out windows/msvc_variants and addressed your feedback @scheibelp. I'll let the tests go again and LMK if you notice anything else out of the ordinary! |
|
#31265 looks related to the style failure, if applying the fix described there works, then could you mention that in that issue? (one thing that's strange: that issue describes how this would come up for non-Ubuntu systems, but I checked the runner for this style check and it appears to be Ubuntu, so I'm not sure how this would come up there - it is the same error message though) |
|
@scheibelp Pinging you as per your requests. I'll look into the conflicts if you want to examine anything else |
scheibelp
left a comment
There was a problem hiding this comment.
I have a few questions/requests. I'll do a more-thorough review once this is updated with the latest develop.
72ea2d7 to
123e7be
Compare
cfe159e to
2755724
Compare
|
@spackbot run pipeline |
|
I've started that pipeline for you! |
|
Thanks @jpopelar! |
|
You're welcome @scheibelp, have a good Thanksgiving |
| # Skip this on Windows since pkgconfig is autotools | ||
| for plat in ["cray", "darwin", "linux"]: | ||
| depends_on("pkgconfig", when="platform=%s" % plat, type="run") |
There was a problem hiding this comment.
I'm late with my comments but nonetheless.
- I disagree with the comment:
pkgconfigis notautotools. - The compiler wrappers will not work without
pkg-configin thePATH.
I understand that the compiler wrappers of hdf5 a rarely used but if pkg-config works on Windows, the code above should be reverted. Otherwise, the wrappers should be removed after the installation to avoid confusion.
* Enable hdf5 build (including +mpi) on Windows * This includes updates to hdf5 dependencies openssl (minor edit) and bzip2 (more-extensive edits) * Add binary-based installation of msmpi (this is currently the only supported MPI implementation in Spack for Windows). Note that this does not install to the Spack-specified prefix. This implementation will be replaced with a source-based implementation Co-authored-by: John Parent <[email protected]>
Remake of PR #29826 due to branch conflicts and duplicated commits