Skip to content

Fix library install directories on Windows#634

Draft
sloede wants to merge 1 commit into
openfheorg:mainfrom
sloede:msl/fix-windows-install-dirs
Draft

Fix library install directories on Windows#634
sloede wants to merge 1 commit into
openfheorg:mainfrom
sloede:msl/fix-windows-install-dirs

Conversation

@sloede

@sloede sloede commented Dec 31, 2023

Copy link
Copy Markdown

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

sloede commented Jan 5, 2024

Copy link
Copy Markdown
Author

From #633 (comment):

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.

@yspolyakov yspolyakov marked this pull request as draft January 16, 2024 20:13
@sloede

sloede commented Mar 6, 2024

Copy link
Copy Markdown
Author

@dsuponitskiy Are there any updates on this issue?

@arcturusannamalai

Copy link
Copy Markdown

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

@sloede

sloede commented Apr 15, 2024

Copy link
Copy Markdown
Author

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.

@arcturusannamalai

Copy link
Copy Markdown

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

@sloede

sloede commented Apr 20, 2024

Copy link
Copy Markdown
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants