Skip to content

Fix couple of items discovered on Windows.#519

Merged
markcmiller86 merged 3 commits intomainfrom
biagas/fix_windows_issues
Mar 24, 2026
Merged

Fix couple of items discovered on Windows.#519
markcmiller86 merged 3 commits intomainfrom
biagas/fix_windows_issues

Conversation

@biagas
Copy link
Copy Markdown
Collaborator

@biagas biagas commented Mar 2, 2026

Location of silo-targets.cmake in the install.
Ensuring HDF5's find of zlib finds the correct one if SILO_ZLIB_DIR set.
Fix what gets deployed on Windows when using windeployqt.

@biagas
Fix couple of items discovered on Windows.
94db26c 
Location of silo-targets.cmake in the install.
Ensuring HDF5's find of zlib finds the correct one if SILO_ZLIB_DIR set.
Fix what gets deployed on Windows when using `windeployqt`.
@biagas biagas requested a review from markcmiller86 March 2, 2026 20:22
biagas added 2 commits March 2, 2026 12:36
@biagas
More fixes from windows.
f99f71d 
Fix zlib target name in SiloFindZlib when trying to copy dll.
Use HDF5_PROVIDES_ZLIB_SUPPORT.
@biagas
Fix how zlib dll is found for install.
7ca9c6e
@ldowen
Copy link
Copy Markdown

ldowen commented Mar 23, 2026

I believe this PR includes a fix needed to get the CMake exports working. The issue is that sometimes, Silo installs it's libraries into lib64 but SiloConfig.cmake is hard-coded to look for lib/cmake/Silo/SiloTargets.cmake.

There is also a small cosmetic inconsistency where the the use of SiloConfig.cmake means users must use find_package(Silo (upper case S) but the exported target is silo (lowercase s). I think making the target Silo and perhaps adding add_library(silo ALIAS Silo) or something for backward compatibility would help.

markcmiller86
markcmiller86 approved these changes Mar 23, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@markcmiller86 markcmiller86 left a comment

Choose a reason for hiding this comment

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

Thanks so much @biagas ❤️

@markcmiller86
Copy link
Copy Markdown
Member

markcmiller86 commented Mar 23, 2026

@junghans do you by any chance know much about the fedpkg process and the need for openh264?

junghans
junghans approved these changes Mar 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@junghans junghans left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Works for me!

@biagas
Copy link
Copy Markdown
Collaborator Author

biagas commented Mar 23, 2026

@markcmiller86 should I be concerned about the failing rpm build checks?

@junghans
Copy link
Copy Markdown
Contributor

junghans commented Mar 23, 2026

@markcmiller86 should I be concerned about the failing rpm build checks?

I think it is just bad timing, I would retrigger tomorrow morning.

@markcmiller86 markcmiller86 merged commit 055732a into main Mar 24, 2026
4 of 6 checks passed
@markcmiller86 markcmiller86 deleted the biagas/fix_windows_issues branch March 24, 2026 01:09
markcmiller86 added a commit that referenced this pull request Mar 24, 2026
@markcmiller86
Merge pull request #519 from llnl/biagas/fix_windows_issues
8b7a82e 
Fix couple of items discovered on Windows.
markcmiller86 added a commit that referenced this pull request Mar 24, 2026
@markcmiller86
Merge pull request #522 from llnl/backport-llnl/biagas/fix_windows_is…
4beb36a 
…sues

Merge pull request #519 from llnl/biagas/fix_windows_issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markcmiller86 markcmiller86 markcmiller86 approved these changes

+1 more reviewer

@junghans junghans junghans approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@biagas @ldowen @markcmiller86 @junghans