openfhe-development icon indicating copy to clipboard operation
openfhe-development copied to clipboard

Fix library install directories on Windows

Open sloede opened this issue 1 year ago • 6 comments

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 to lib, while the dynamic libraries go to bin

sloede avatar Dec 31 '23 12:12 sloede

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.

sloede avatar Jan 05 '24 09:01 sloede

@dsuponitskiy Are there any updates on this issue?

sloede avatar Mar 06 '24 13:03 sloede

@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()

arcturusannamalai avatar Apr 15 '24 05:04 arcturusannamalai

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.

sloede avatar Apr 15 '24 07:04 sloede

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

arcturusannamalai avatar Apr 15 '24 15:04 arcturusannamalai

@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)

sloede avatar Apr 20 '24 07:04 sloede