openfhe-development
openfhe-development copied to clipboard
Fix library install directories on Windows
Currently, OpenFHE installs all library products in the lib
directory. AFAIK, this is wrong (or at least bad practice) on Windows, where the .dll
files belong into the bin
directory.
This PR remedies this by configuring the library installation in CMake to do The Right Thing:
- On *nix systems, all library products go to
lib
- On Windows systems (including Cygwin and MinGW), static libraries and
.dll.a
or.lib
libraries go tolib
, while the dynamic libraries go tobin
From https://github.com/openfheorg/openfhe-development/pull/633#issuecomment-1877451241:
At the moment I am not sure how we are going to proceed with PR #634. I would need to discuss this with my colleague(s) first.
@dsuponitskiy Sure, let me know if I can be of any help. Right now, I don't see any downsides to the proposed change, since it essentially leaves all non-DLL systems unchanged and fixes only the install locations for DLL systems.
Note that the proposed change is also the default for the currently required minimum CMake version 3.5.2, while at least since CMake version 3.14 it is possible to change the install location ad hoc during configure time using the CMAKE_INSTALL_XXX
variables.
@dsuponitskiy Are there any updates on this issue?
@sloede - you may want to change your code to be conditional to WINDOWS e.g. see: https://stackoverflow.com/questions/9160335/os-specific-instructions-in-cmake-how-to
if(MSVC OR MSYS OR MINGW)
# for detecting Windows compilers
endif()
you may want to change your code to be conditional to WINDOWS
I do not think that's necessary. The proposed parameter settings should work for all Unix-like and Windows-like systems. In fact, I think it has been the default for a number of CMake releases now.
you may want to change your code to be conditional to WINDOWS
I do not think that's necessary. The proposed parameter settings should work for all Unix-like and Windows-like systems. In fact, I think it has been the default for a number of CMake releases now.
good to know; thanks
@yspolyakov are there any updates on this issue? I'd be happy to update the PR such that it can be merged eventually (I don't think there is much to it anyways)