Allow Windows install() to use GNUInstallDirs structure - #1518
Merged
Conversation
zaddach
force-pushed
the
simplify_windows_install
branch
5 times, most recently
from
June 19, 2025 13:32
8586a24 to
f9ac05b
Compare
zaddach
force-pushed
the
simplify_windows_install
branch
4 times, most recently
from
August 7, 2025 11:58
0e5ba7d to
4bc3f10
Compare
Contributor
Author
|
Renders #1524 obsolete. |
Contributor
|
I started review but haven't had time to finish, yet. It won't make it into the clamav 1.5.0-rc. I will try to finish my review soon and if the changes aren't too extensive maybe we can put it in before the 1.5.0 stable release in ~1-2 weeks. |
val-ms
requested changes
Sep 28, 2025
val-ms
left a comment
val-ms
left a comment
Contributor
There was a problem hiding this comment.
Overall this is really good work. I have a few change requests to improve it. Thank you for your help and for figuring out the CMakePresets.json.
zaddach
force-pushed
the
simplify_windows_install
branch
2 times, most recently
from
October 20, 2025 10:14
11d6988 to
9c03a4d
Compare
|
this would be cool, merge when |
The install BINDIR, SBINDIR, LIBDIR, and INCLUDEDIR may be set in the top level CMakeLists.txt and does not need to be set for each target. This also enables a build to customize those so that the bins COULD go to the "bin" directory. Also add CMake option "ENABLE_WINDOWS_INSTALL_THIRDPARTY_DEPENDENCIES". - When enabled (default), the install will include required system runtime libraries and also a copy of the other third party library dependencies (e.g. libpcre2, libxml2, libcrypto, etc.). - When disabled, those files are not included. One situation where this is useful is if vcpkg is being used to build ClamAV itself (including its dependencies).
val-ms
force-pushed
the
simplify_windows_install
branch
from
December 12, 2025 23:48
26bda09 to
751ab78
Compare
Contributor
|
I just rebased the branch and combined some of the commits to tidy up the history and put in some more high level descriptions for each commit. The content of the PR is identical to before my force-push. |
Build on windows-2022, as windows-2019 is no longer available Use updated vcpkg github action Try to use vcpkg manifest mode Provide vcpkg baseline commit hash Correctly request dependency feature Fix printing vcpkg triplet Try building with CMakePresets.json
In inflate64.c on Windows, as zlib.h is included, it sets ZEXTERN to " __declspec(dllimport)". In consequence, our internal functions have the wrong linkage and linking fails because of the missing dll functions.
The prescan and file_inspection callbacks are deprecated and should not be used. They are replaced by a more unified scan callback system which is demonstrated in ex_scan_callbacks.c.
Don't concatenate pythonpath and str Fix test program path for Windows Ninja Multi-Config Use environment variable to pass cl_cvdunpack_test location to test Use ENABLE_EXAMPLES instead of ENABLE_TESTS to gate ex_cl_cvdunpack Directly use the target's path on Windows Also change the program finding for ex_scan_callbacks_test Fix python paths handling Fix the order of some expected results which were incorrect and for some reason only failed when tested on Windows. This final fix in this commit provided by Val Snyder.
val-ms
force-pushed
the
simplify_windows_install
branch
from
December 15, 2025 19:14
79afe16 to
661826d
Compare
Contributor
|
I enabled verbose ctest output and fixed installing a couple of the example programs which was causing windows tests to fail. Looks good to me, so I squashed those changes into the commit that fixed the github actions windows workflow and force-pushed it. |
val-ms
approved these changes
Dec 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Right now ClamAV will install binary products in a flat directory structure on Windows. We're trying to build a conan package from the built artifacts, and it would be handy to keep the GNUInstallDirs (lib/, bin/, include/, ... ) directory structure.
This PR doesn't handle Windows install in a special way any more, but provides defaults (that default to the same behavior as before) for the GNUInstallDirs, while allowing them to be overridden by the user.
Further, it introduces an option to install runtime dependencies. The option is on by default to mimick the current behavior, and can be switched off if you don't need dependencies and system DLLs installed.